-
Notifications
You must be signed in to change notification settings - Fork 571
feat: add route search form with 9 filter fields #3222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Created SearchForm component with name, id, host, path, description, plugin, labels, version, status filters - Implemented hybrid filtering (backend for name, client-side for others) - Added client-side filtering utilities in clientSideFilter.ts - Integrated search form into routes list page - Extended pageSearch schema to support new filter parameters - Added translations for en, es, de, zh locales - Fixed clsx import warning in Editor.tsx This enables users to quickly find target routes when managing hundreds of routes in a cluster. Closes apache#3205
Hi @DSingh0304, thanks for your contribution, we need to add tests for this functionality. |
Well I can do that too need some time for this though... |
Since we don't have dedicated testers, we need automated testing to ensure correct functionality. You can refer to the existing test cases in the e2e directory. |
Okay sure I will go through it ... |
@Baoyuantop I have written the test and it's working fine. Please check once ... |
Hi, pls resolve conflicts, and merge master, thx 😸 |
Hi @SkyeYoung & @Baoyuantop, I have resolved the conflict ... |
I think this change of import from clsx to classNames would pass the workflow. |
"name": "Name", | ||
"id": "ID", | ||
"host": "Host", | ||
"path": "Path", | ||
"description": "Description", | ||
"plugin": "Plugin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the placeholder just repeats the content of the label, then I don't think it's necessary to have a placeholder. My suggestion is to remove it instead.
const handleSearch = (values: SearchFormValues) => { | ||
// Send name filter to backend, keep others for client-side filtering | ||
setParams({ | ||
page: 1, | ||
name: values.name, | ||
id: values.id, | ||
host: values.host, | ||
path: values.path, | ||
description: values.description, | ||
plugin: values.plugin, | ||
labels: values.labels, | ||
version: values.version, | ||
status: values.status, | ||
}); | ||
}; | ||
|
||
const handleReset = () => { | ||
setParams({ | ||
page: 1, | ||
name: undefined, | ||
id: undefined, | ||
host: undefined, | ||
path: undefined, | ||
description: undefined, | ||
plugin: undefined, | ||
labels: undefined, | ||
version: undefined, | ||
status: undefined, | ||
}); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to make it less bloated? 🤔
// Filter by plugin | ||
if (filters.plugin && routeData.plugins) { | ||
const pluginNames = Object.keys(routeData.plugins).join(',').toLowerCase(); | ||
const pluginMatch = pluginNames.includes(filters.plugin.toLowerCase()); | ||
if (!pluginMatch) return false; | ||
} | ||
|
||
// Filter by labels | ||
if (filters.labels && filters.labels.length > 0 && routeData.labels) { | ||
const routeLabels = Object.keys(routeData.labels).map((key) => | ||
`${key}:${routeData.labels![key]}`.toLowerCase() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best to comment on the effects of these methods to prevent future maintainers from misunderstanding them.
Description
Adds a comprehensive search form to the routes list page, enabling users to quickly find target routes when managing hundreds of routes in a cluster.
Changes
SearchForm
component with 9 filter fields:name
parameter (supported by APISIX API)Testing
Screenshots
Closes #3205